[Spec 1273][Verify] Fix afx reset: own-project resolution + task lane - #1308
Merged
Conversation
… support the task lane Verify-phase e2e found afx reset non-functional in production on every lane. Three fixes, all confirmed against live builders with --dry-run. F1 (BLOCKER, all lanes) — wrong-winner project selection. readPorchContext returned the FIRST directory under codev/projects with a parsable status.yaml. Correct for a repo with one project dir; wrong here, because porch history is committed to main and every worktree inherits every project ever run — 203 of them. The alphabetically-first is 0087-porch-timeout-termination-retries, protocol `spider`, so EVERY builder resolved protocol spider and died on `Protocol "spider" has no builder-prompt.md`. The comment I wrote at context.ts:163 — "when a worktree somehow holds more than one project directory" — named the assumption that broke. It was not "somehow"; it is the normal state of every worktree in this repo. This is the #1235 wrong-winner family: pick-the-first over a set assumed to be a singleton. Now selects by identity, never by position. readPorchContext takes a PorchProjectIdentity and matches status.yaml's `id` (or the directory prefix) against the issue number, the builder id, or the builder id minus its protocol prefix. Comparison ignores case and leading zeros — the registry lowercases ids while directories preserve case (builder-task-re_v vs .builders/task-RE_V, which macOS hides and Linux would not), and dirs use 0087 where ids use 87. No match returns null. That is the honest answer for a genuinely non-porch builder, and it is also the safe one for a porch builder: a missing porch block is visible in the re-orientation, whereas another project's protocol is a confident lie. porch is now resolved ONCE and reused for both the protocol chain and the result, so two scans cannot disagree. F1b — the bare --task lane has no protocol template. With F1 fixed, the probe resolved protocol `task` from its builder id and died on `Protocol "task" has no builder-prompt.md`. `afx spawn --task` without --protocol takes spawn.ts's else-branch (:551) and builds the prompt straight from the task text; there is no protocols/task/builder-prompt.md to render. buildLongForm now reproduces that shape for the bare-task lane, selected on a POSITIVE fact (taskText present and no porch) rather than by catching the error. The --task --protocol variant does get a porch project (:548), so it is unaffected and still renders its real template. Header, state pointer, addendum and porch re-entry moved into a shared assembleLongFormDocument so the framing cannot drift between lanes. F2 — the task lane could never auto-detect its mode. Mode detection requires a `## Mode:` line, which task spawns never render, so afx reset <task> hard-errored without --mode. A no-porch builder now defaults to soft with modeSource 'task-default'. Not a guess dressed as a fact: "strict" means porch orchestrates, and a builder with no porch project cannot be strict. The source is recorded so the report says where it came from. Scoped to the no-porch case — a porch builder missing its Mode line is a genuine ambiguity and still aborts, now naming the project that proves it is not the task lane. F3 — reported, not a defect in reset. The path printed (.builders/task-RE_V) is CORRECT and comes verbatim from the registry, which stores id `builder-task-re_v` alongside worktree `.builders/task-RE_V`. The id/directory case divergence originates at spawn, not here. Reset was already using registry paths verbatim; the new id matching is case-insensitive so a case-sensitive filesystem behaves like macOS. Verified live with --dry-run (zero writes): task-re_v -> TASK/soft with spawn's task prompt reproduced verbatim; 1286 -> ASPIR/strict on project 1286-consult-configurable-per-lane-; 1273 -> ASPIR. No spider anywhere. Tests: multi-project-dir fixture reproducing the 203-dir shape (own project wins over three foreign spider projects; null when the builder owns none; issue-number match; leading-zero and case tolerance), task-lane mode default plus the porch-builder abort it must not swallow, and two reorient tests pinning that the template port is never called for a bare task and still is for --task --protocol. Suite 4016 -> 4027, build clean.
Verify-PR CMAP iter 1: Gemini APPROVE, Claude REQUEST_CHANGES, Gemini/Codex lane missing. Claude's finding accepted — a real regression introduced by my own F1 fix. candidateProjectIds stripped the `builder-` prefix and offered only the stripped forms. But `spawn --task --protocol X` passes the FULL builderId to porch init (spawn.ts:548), and initPorchInWorktree keeps dashes when sanitising (spawn-worktree.ts:472), so that lane's porch project id really is `builder-task-<id>`. No candidate matched it, so readPorchContext returned null and the builder was re-oriented as protocol TASK with no porch re-entry. That failure mode is worse than the one F1 fixed: `spider` aborted loudly and touched nothing, whereas this silently produced a DEGRADED frame — a porch-driven builder told it had no porch. Exactly the silent-drift class R3 exists to prevent, reintroduced by the fix for a loud bug. Narrowing a match is as dangerous as widening it. Now offers the raw registry id as a candidate as well as the stripped form. Tests: the porch id with a raw `builder-` prefix resolves, and the lane end-to-end keeps protocolSource 'status.yaml' and modeSource 'builder-prompt' (NOT the task default — this lane has porch, so the F2 default must not fire for it). Live re-verified, unchanged: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK. Suite 4027 -> 4029, build clean.
… project id Verify-PR CMAP iter 2: Gemini APPROVE, Claude COMMENT (one item flagged worth fixing before merge), Codex lane unavailable. Accepted. `issueNumber: issueNumber ?? porch?.projectId` assumed every porch project id is an issue number. For the `--task --protocol` lane the previous commit just un-orphaned, that id is `builder-task-<id>`, so the re-orientation rendered `- Issue: #builder-task-abc` and an unfollowable `gh issue view builder-task-abc`. A fabricated issue reference is worse than no issue line: on issue-driven protocols the issue body IS the spec, so this sends a freshly-reset builder — one with no history to notice the oddity — to look up requirements that do not exist. Now guarded with /^\d+$/; a non-numeric project id yields undefined and the frame simply omits the issue. Pinned in the existing --task --protocol end-to-end test rather than a new one, so the lane's expectations live together. Live re-verified, unchanged: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK. Suite 4029, build clean.
Verify-PR CMAP iter 3: Gemini APPROVE, Claude REQUEST_CHANGES (verified live).
Accepted — the wrong-winner class SURVIVED my first fix.
F1 stopped alphabetically-first from winning, but a bare-NUMBER match still let
one project claim any builder sharing the digits, across protocols. Confirmed
in this repo: issue 799 has a PIR project
(799-vscode-builder-changed-file-ro, protocol pir) and a bugfix, so
`builder-bugfix-799` silently adopted the PIR project's protocol and porch id.
Silent, not loud — the degraded-frame failure mode, worse than the `spider`
abort it replaced.
Claims are now graded:
strong — the project id equals the raw registry builder id and is NON-numeric
(`builder-task-abc`). Globally unique, so it needs no corroboration
— which matters because that lane's protocol legitimately differs
from its id prefix (task vs air).
weak — matched on the bare number or a directory prefix. Requires the
project's protocol to AGREE with the protocol the builder id
declares.
none — not this builder's.
Two or more surviving claims abort loudly rather than picking one. Choosing the
least-wrong of several is the original bug with better manners.
DELIBERATE SEMANTIC REVERSAL, called out because it changes phase-4 behaviour
and breaks a test I wrote there: "prefers status.yaml over the builder id when
they disagree" no longer holds for the PROTOCOL field. status.yaml remains
authoritative for a builder's own project (phase, plan phase, identity), but it
can no longer DECIDE which project is its own. The two principles cannot both
hold — the signal is identical in each case — and the live harm settles it: a
hypothetical id/status protocol disagreement within one project is the cost;
adopting a stranger's project is what is actually happening here. The test now
encodes the new rule with that reasoning.
Tests: the 799 collision (bugfix builder gets null, PIR builder gets the
project, and the bugfix builder still resolves protocol from its id with no
porch block — visible absence, not a confident lie), the non-numeric id
crossing a protocol mismatch, and the two-equal-claims abort. Three earlier
fixtures paired a builder with a foreign-protocol project and were unrealistic;
rebuilt protocol-consistent so they still exercise zero/case tolerance.
Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4029 -> 4035, build clean.
waleedkadous
added a commit
that referenced
this pull request
Jul 31, 2026
…iew of #1308 #1308 merged on 2-way CMAP during the codex outage. Retroactive advisory review (architect-directed) returned REQUEST_CHANGES with real findings. Four fixed here; two recorded as pre-existing and not fixed. 1. A status.yaml stating a DIFFERENT id was overruled by its directory name. `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever the directory is called. The dir-name fallback let a renamed or recycled directory claim a builder, and manufactured false ambiguities beside the real project. The fallback now applies ONLY when the file states no id. 2. Weak claims survived when corroboration was impossible. The code read `if (expectedProtocol && mismatch) continue`, so a noncanonical builder id — which yields no protocol — let EVERY weak claim through. My own comment two lines above said such a claim "cannot be corroborated and is not trusted". The comment was right and the code did the opposite; now it rejects. (Third time this project that a docstring asserted what the code did not do.) 3. The numeric issueNumber guard dropped BUGFIX issue identity. BUGFIX stores porch ids as `bugfix-<N>` deliberately (spawn.ts:817, "historical, kept untouched"), so /^\d+$/ discarded it whenever the registry row had none — and on BUGFIX the issue body IS the spec, so the re-orientation lost the requirements it exists to carry. This was a regression I introduced in #1308's own issueNumber fix. New issueNumberFromPorchId accepts a bare number or a canonical `<word>-<number>`, still rejecting ad-hoc task ids. 4. Bare-task detection was a negative inference. initPorchInWorktree is deliberately non-fatal, so a `--task --protocol X` builder whose porch init FAILED also has task text and no porch — and `taskText && !porch` stripped its real protocol template and handed it raw task text instead. Replaced with positive evidence: `isBareTask` on the resolved context, true only when task text is present AND the prompt carries no `## Mode:` heading. The prompt is written BEFORE porch init (spawn.ts:545-548), so a --task --protocol builder's rendered template survives a failed init and is reliable evidence. This also correctly re-scopes the F2 mode default. RECORDED, NOT FIXED — both pre-existing spawn properties rather than regressions from this work, and neither is reachable in this repo today: - Hyphenated protocol names: `builder-my-protocol-1273` parses as protocol `my` via parseAgentName's first-dash split. Needs a custom protocol containing a hyphen; none exists. Fixing it means constraining protocol names at spawn or changing the agent-name encoding — outside a reset fix. - Task short ids are 24-bit random and not checked against committed porch history, so a reused `builder-task-xxxx` could strong-match a stale project. Probabilistic and spawn-side; belongs with the id-generation issue (#1312). Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK. Suite 4035 -> 4041, build clean.
waleedkadous
added a commit
that referenced
this pull request
Jul 31, 2026
…iew of #1308 #1308 merged on 2-way CMAP during the codex outage. Retroactive advisory review (architect-directed) returned REQUEST_CHANGES with real findings. Four fixed here; two recorded as pre-existing and not fixed. 1. A status.yaml stating a DIFFERENT id was overruled by its directory name. `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever the directory is called. The dir-name fallback let a renamed or recycled directory claim a builder, and manufactured false ambiguities beside the real project. The fallback now applies ONLY when the file states no id. 2. Weak claims survived when corroboration was impossible. The code read `if (expectedProtocol && mismatch) continue`, so a noncanonical builder id — which yields no protocol — let EVERY weak claim through. My own comment two lines above said such a claim "cannot be corroborated and is not trusted". The comment was right and the code did the opposite; now it rejects. (Third time this project that a docstring asserted what the code did not do.) 3. The numeric issueNumber guard dropped BUGFIX issue identity. BUGFIX stores porch ids as `bugfix-<N>` deliberately (spawn.ts:817, "historical, kept untouched"), so /^\d+$/ discarded it whenever the registry row had none — and on BUGFIX the issue body IS the spec, so the re-orientation lost the requirements it exists to carry. This was a regression I introduced in #1308's own issueNumber fix. New issueNumberFromPorchId accepts a bare number or a canonical `<word>-<number>`, still rejecting ad-hoc task ids. 4. Bare-task detection was a negative inference. initPorchInWorktree is deliberately non-fatal, so a `--task --protocol X` builder whose porch init FAILED also has task text and no porch — and `taskText && !porch` stripped its real protocol template and handed it raw task text instead. Replaced with positive evidence: `isBareTask` on the resolved context, true only when task text is present AND the prompt carries no `## Mode:` heading. The prompt is written BEFORE porch init (spawn.ts:545-548), so a --task --protocol builder's rendered template survives a failed init and is reliable evidence. This also correctly re-scopes the F2 mode default. RECORDED, NOT FIXED — both pre-existing spawn properties rather than regressions from this work, and neither is reachable in this repo today: - Hyphenated protocol names: `builder-my-protocol-1273` parses as protocol `my` via parseAgentName's first-dash split. Needs a custom protocol containing a hyphen; none exists. Fixing it means constraining protocol names at spawn or changing the agent-name encoding — outside a reset fix. - Task short ids are 24-bit random and not checked against committed porch history, so a reused `builder-task-xxxx` could strong-match a stale project. Probabilistic and spawn-side; belongs with the id-generation issue (#1312). Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK. Suite 4035 -> 4041, build clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The verify-phase e2e found
afx resetnon-functional in production on every lane. This fixes it. All three fixes are confirmed against live builders with--dry-run(zero writes).Follow-up to #1305 under the same pre-grant terms.
Refs #1273
F1 (BLOCKER, all lanes) — wrong-winner project selection
readPorchContextreturned the first directory undercodev/projectswith a parsablestatus.yaml. Correct for a repo with one project dir; wrong here, because porch history is committed tomainand every worktree inherits every project ever run — 203 of them. The alphabetically-first is0087-porch-timeout-termination-retries, protocolspider, so every builder resolved protocolspiderand died onProtocol "spider" has no builder-prompt.md.The comment I had written at
context.ts:163— "when a worktree somehow holds more than one project directory" — named the assumption that broke. It was not "somehow"; it is the normal state of every worktree in this repo. This is the #1235 wrong-winner family: pick-the-first over a set assumed to be a singleton.Now selects by identity, never by position. Matching ignores case and leading zeros — the registry lowercases ids while directories preserve case (
builder-task-re_vvs.builders/task-RE_V, which macOS hides and Linux would not), and dirs use0087where ids use87.No match returns
null— the honest answer for a non-porch builder, and the safe one for a porch builder: a missing porch block is visible in the re-orientation, whereas another project's protocol is a confident lie.F1b — the bare
--tasklane has no protocol templateWith F1 fixed, the probe resolved protocol
taskand died onProtocol "task" has no builder-prompt.md.afx spawn --taskwithout--protocolbuilds its prompt straight from the task text (spawn.ts:551); there is no template to render. The long form now reproduces that shape, selected on a positive fact (task text present, no porch) rather than by catching the error. The--task --protocolvariant does get a porch project, so it still renders its real template.F2 — the task lane could never auto-detect its mode
Mode detection requires a
## Mode:line that task spawns never render, soafx reset <task>hard-errored without--mode. A no-porch builder now defaults to soft withmodeSource: 'task-default'. Not a guess dressed as a fact: strict means porch orchestrates, and a builder with no porch project cannot be strict. Scoped to the no-porch case — a porch builder missing its Mode line is a genuine ambiguity and still aborts.F3 — reported, but not a defect in reset
The path printed (
.builders/task-RE_V) is correct and comes verbatim from the registry, which stores idbuilder-task-re_valongside worktree.builders/task-RE_V. The id/directory case divergence originates at spawn, not here. Reset already used registry paths verbatim; the new id matching is case-insensitive so a case-sensitive filesystem behaves like macOS. Happy to file the spawn-side normalization separately if wanted.Verification
Live
--dry-runagainst real builders:task-re_vProtocol "spider" has no builder-prompt.mdTASK/soft, spawn's task prompt reproduced verbatim1286Protocol "spider" …ASPIR/strict, project1286-consult-configurable-per-lane-1273Protocol "spider" …ASPIRSuite 4016 → 4027, build clean.
New tests: a multi-project-dir fixture reproducing the 203-dir shape (own project wins over three foreign
spiderprojects;nullwhen the builder owns none; issue-number match; leading-zero and case tolerance), the task-lane mode default plus the porch-builder abort it must not swallow, and two reorient tests pinning that the template port is never called for a bare task and still is for--task --protocol.What the e2e vindicated
Every failure was a refusal, not a corruption — the R1–R4 invariants did their job,
--dry-runwrote nothing as promised, and no builder ever received a wrong re-orientation. The bugs were all in resolution, upstream of the destructive path.Still outstanding: the live reset against the planted-context probe, for the architect to re-run after this installs.